Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: calculate the io throughput in background in ReadLimiter #5415

Merged
merged 20 commits into from
Aug 4, 2022
Merged

feat: calculate the io throughput in background in ReadLimiter #5415

merged 20 commits into from
Aug 4, 2022

Conversation

Lloyd-Pottiger
Copy link
Contributor

@Lloyd-Pottiger Lloyd-Pottiger commented Jul 19, 2022 •
edited
Loading

Signed-off-by: Lloyd-Pottiger yan1579196623@gmail.com

What problem does this PR solve?

Issue Number: close #5091, close #5401

Problem Summary:

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Test

Test step

  1. Limit io throughput by following configuration:
max_bytes_per_sec = 400 * 1024 * 1024
foreground_write_weight = 25
background_write_weight = 25
foreground_read_weight = 25
background_read_weight = 25
  1. Clear page cache
sync
echo 3 > /proc/sys/vm/drop_caches
  1. Run
select count(L_ORDERKEY), count(L_PARTKEY), count(L_SUPPKEY), count(L_LINENUMBER), count(L_QUANTITY), count(L_EXTENDEDPRICE), count(L_DISCOUNT), count(L_TAX), count(L_RETURNFLAG), count(L_LINESTATUS), count(L_SHIPDATE), count(L_COMMITDATE), count(L_RECEIPTDATE), count(L_SHIPINSTRUCT), count(L_SHIPMODE), count(L_COMMENT) from tpch_100.lineitem;

In TPC-H-100 dataset.
4. Record the CPU Usage, I/O Limiter and Disk Throughput in grafana.

Note: in order to view the result more intuitively, we turn off auto_tune.

Test Result

update_io_stat_period_ms = 20
image
image
image
update_io_stat_period_ms = 50
image
image
image
update_io_stat_period_ms = 100
image
image
image
update_io_stat_period_ms = 200
image
image
image
update_io_stat_period_ms = 2000
image
image
image

Analysis

The quota's refresh cycle is 100ms, and in order to avoid read_bytes not being refreshed for the whole cycle, the update_io_stat_period_ms should be less than 100ms. When update_io_stat_period_ms = 50, read_bytes may be refreshed only once. We suggest setting update_io_stat_period_ms to 30ms, which makes sure read_bytes is refreshed three times in one cycle and the CPU usage is acceptable.

Release note

Calculate the io throughput in background in ReadLimiter

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jul 19, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • JinheLin
  • lidezhu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 19, 2022
@Lloyd-Pottiger Lloyd-Pottiger marked this pull request as draft July 19, 2022 10:47
@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 19, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 19, 2022

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp             525               242    53.90%          44                11    75.00%         661               245    62.93%         284               131    53.87%
Encryption/RateLimiter.h                76                10    86.84%          29                 3    89.66%         114                31    72.81%          38                12    68.42%
Interpreters/Context.cpp               533               330    38.09%         169                88    47.93%        1154               701    39.25%         290               216    25.52%
Server/StorageConfigParser.cpp         407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                 1541               671    56.46%         265               103    61.13%        2354              1000    57.52%         848               444    47.64%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18618      9407             49.47%    212174  94946        55.25%

full coverage report (for internal network access only)

@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 21, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 21, 2022

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
RateLimiter.cpp                   550               253    54.00%          48                13    72.92%         716               272    62.01%         300               141    53.00%
RateLimiter.h                      68                 2    97.06%          28                 2    92.86%         103                20    80.58%          32                 6    81.25%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             618               255    58.74%          76                15    80.26%         819               292    64.35%         332               147    55.72%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18621      9406             49.49%    212217  94929        55.27%

full coverage report (for internal network access only)

@Lloyd-Pottiger
fix issue 5401
4b36863 
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@Lloyd-Pottiger
run getCurrentIOInfo in auto_tune_thread
d5c3c4a 
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 21, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/run-all-tests

@Lloyd-Pottiger
fix unit test
8fa8611 
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@Lloyd-Pottiger Lloyd-Pottiger changed the title feat: calculate the io throughput in background in IOLimiter feat: calculate the io throughput in background in ReadLimiter Jul 22, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 22, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      535               257    51.96%          45                13    71.11%         694               288    58.50%         298               147    50.67%
Encryption/RateLimiter.h                         67                 2    97.01%          27                 2    92.59%         100                20    80.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp        3156              1033    67.27%          38                 0   100.00%         686                20    97.08%         818               420    48.66%
Server/StorageConfigParser.cpp                  407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          4167              1381    66.86%         135                16    88.15%        1910               351    81.62%        1384               658    52.46%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18616      9406             49.47%    212180  94969        55.24%

full coverage report (for internal network access only)

@Lloyd-Pottiger
Copy link
Contributor Author

/run-integration-test

JinheLin
JinheLin reviewed Jul 25, 2022
View reviewed changes
dbms/src/Encryption/RateLimiter.cpp Outdated Show resolved Hide resolved
dbms/src/Encryption/RateLimiter.cpp Outdated Show resolved Hide resolved
dbms/src/Encryption/RateLimiter.cpp Outdated Show resolved Hide resolved
dbms/src/Encryption/RateLimiter.cpp Show resolved Hide resolved
dbms/src/Encryption/RateLimiter.cpp Outdated Show resolved Hide resolved
@Lloyd-Pottiger
avoid meanless calculation
e304dea 
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@Lloyd-Pottiger
Copy link
Contributor Author

/run-unit-test

@sre-bot
Copy link
Collaborator

sre-bot commented Aug 1, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      534               257    51.87%          45                13    71.11%         693               290    58.15%         296               146    50.68%
Encryption/RateLimiter.h                         67                 3    95.52%          27                 3    88.89%         100                23    77.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp         307                60    80.46%          38                 0   100.00%         686                20    97.08%          56                10    82.14%
Server/StorageConfigParser.cpp                  407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          1317               409    68.94%         135                17    87.41%        1909               356    81.35%         620               247    60.16%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18858      9434             49.97%    214988  95352        55.65%

full coverage report (for internal network access only)

@Lloyd-Pottiger
rename some variables and change default value to 30
820766b 
Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>
@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Aug 3, 2022
@Lloyd-Pottiger Lloyd-Pottiger marked this pull request as ready for review August 3, 2022 08:46
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Aug 3, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      534               257    51.87%          45                13    71.11%         693               290    58.15%         296               146    50.68%
Encryption/RateLimiter.h                         67                 3    95.52%          27                 3    88.89%         100                23    77.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp         307                59    80.78%          38                 0   100.00%         686                19    97.23%          56                 9    83.93%
Server/StorageConfigParser.cpp                  407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          1317               408    69.02%         135                17    87.41%        1909               355    81.40%         620               246    60.32%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18858      9430             49.99%    214988  95284        55.68%

full coverage report (for internal network access only)

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 4, 2022
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 4, 2022
@Lloyd-Pottiger
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

@Lloyd-Pottiger: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 820766b

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 4, 2022
@sre-bot
Copy link
Collaborator

sre-bot commented Aug 4, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      534               257    51.87%          45                13    71.11%         693               290    58.15%         296               146    50.68%
Encryption/RateLimiter.h                         67                 3    95.52%          27                 3    88.89%         100                23    77.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp         307                61    80.13%          38                 0   100.00%         686                20    97.08%          56                10    82.14%
Server/StorageConfigParser.cpp                  407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          1317               410    68.87%         135                17    87.41%        1909               356    81.35%         620               247    60.16%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18858      9433             49.98%    214988  95328        55.66%

full coverage report (for internal network access only)

@sre-bot
Copy link
Collaborator

sre-bot commented Aug 4, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      534               257    51.87%          45                13    71.11%         693               290    58.15%         296               146    50.68%
Encryption/RateLimiter.h                         67                 3    95.52%          27                 3    88.89%         100                23    77.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp         307                60    80.46%          38                 0   100.00%         686                20    97.08%          56                10    82.14%
Server/StorageConfigParser.cpp                  407                89    78.13%          23                 1    95.65%         425                23    94.59%         236                85    63.98%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          1317               409    68.94%         135                17    87.41%        1909               356    81.35%         620               247    60.16%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18858      9430             49.99%    214988  95267        55.69%

full coverage report (for internal network access only)

@Lloyd-Pottiger
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

@Lloyd-Pottiger: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@sre-bot
Copy link
Collaborator

sre-bot commented Aug 4, 2022

Coverage for changed files

Filename                                    Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Encryption/RateLimiter.cpp                      613               288    53.02%          45                13    71.11%         693               290    58.15%         272               132    51.47%
Encryption/RateLimiter.h                         67                 3    95.52%          27                 3    88.89%         100                23    77.00%          32                 6    81.25%
Encryption/tests/gtest_rate_limiter.cpp         307                60    80.46%          38                 0   100.00%         686                20    97.08%          56                10    82.14%
Server/StorageConfigParser.cpp                  470                72    84.68%          23                 1    95.65%         425                23    94.59%         216                75    65.28%
TestUtils/MockReadLimiter.h                       2                 0   100.00%           2                 0   100.00%           5                 0   100.00%           0                 0         -
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                          1459               423    71.01%         135                17    87.41%        1909               356    81.35%         576               223    61.28%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
19019      9232             51.46%    215493  94027        56.37%

full coverage report (for internal network access only)

@ti-chi-bot ti-chi-bot merged commit 0fb8f41 into pingcap:master Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidezhu lidezhu lidezhu approved these changes

@JinheLin JinheLin JinheLin approved these changes

Assignees
No one assigned
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
5 participants
@Lloyd-Pottiger @ti-chi-bot @sre-bot @JinheLin @lidezhu